test: add exhaustive AAAT unit tests for GeneralUpdate.Core (Round 4) - #434
Merged
Conversation
Add 20 new test files covering previously untested components: - Configuration: BlackListConfigBuilder, ComparisonResult, UpdateOptionValue, UpdateOptions defaults, HubConfig, Packet, VersionOSS, VersionRespDTO, BaseConfigInfo, ConfigurationMapper edge cases - Download: DefaultDownloadPipeline (SHA256 verify), DownloadProgressReporter, DownloadAsset/Plan/Progress/Result models, PacketDTO/VersioRequest/VersionResponse - FileSystem: FileTreeDiffer (ProduceDeltaPaths, ProduceDeletes, ShouldUseDeltaPatching), FileTreeSnapshot edge cases, BlackListDefaults - Pipeline: PatchMiddleware (null differ skip, success, exception propagation) - Tracer: GeneralTracer (all log levels, toggle, dispose), TextTraceListener All 958 tests pass with 0 failures. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds new xUnit test coverage for previously untested areas of GeneralUpdate.Core, with a focus on AAA-style unit tests and branch/edge-case coverage for configuration models, download pipeline/reporting, file-system diff/snapshotting, pipeline middleware, and tracing utilities.
Changes:
- Added new unit tests for configuration DTOs/options/mappers and blacklist builder/defaults.
- Added new unit tests for download DTOs/models, progress reporting event dispatch, and SHA256 verification pipeline behavior.
- Added new unit tests for file snapshot/diff logic, patch middleware behavior, and tracer/listener lifecycle/concurrency.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/CoreTest/Tracer/TextTraceListenerTests.cs | Exercises TextTraceListener write/write-line behavior, disposal, and concurrent writes. |
| tests/CoreTest/Tracer/GeneralTracerTests.cs | Exercises GeneralTracer log methods, enable/disable toggle, and dispose behavior. |
| tests/CoreTest/Pipeline/PatchMiddlewareTests.cs | Covers PatchMiddleware skip/invoke paths and exception propagation. |
| tests/CoreTest/FileSystem/FileTreeSnapshotExtendedTests.cs | Adds edge-case coverage for FileTreeSnapshot ctor/factories and enumeration. |
| tests/CoreTest/FileSystem/FileTreeDifferExtendedTests.cs | Adds coverage for delta path/delete production and delta-patching decision logic. |
| tests/CoreTest/FileSystem/BlackListDefaultsTests.cs | Verifies BlackListDefaults static default contents and identity. |
| tests/CoreTest/Download/PacketDTOTests.cs | Covers download DTO defaults, assignment, and nullable/tri-state fields. |
| tests/CoreTest/Download/DownloadProgressReporterTests.cs | Covers callbacks + EventManager dispatch for progress/completed/failed/all-complete. |
| tests/CoreTest/Download/DownloadModelsTests.cs | Covers value semantics/defaults for download model record types/enums. |
| tests/CoreTest/Download/DefaultDownloadPipelineTests.cs | Covers DefaultDownloadPipeline hash verification and error/cancellation cases. |
| tests/CoreTest/Configuration/VersionRespDTOTests.cs | Covers VersionRespDTO / BaseResponseDTO<T> defaults and assignment. |
| tests/CoreTest/Configuration/VersionOSSTests.cs | Covers VersionOSS property defaults and assignments, including DateTime extremes. |
| tests/CoreTest/Configuration/UpdateOptionValueTests.cs | Covers UpdateOptionValue<T> construction, boxing, ToString, and edge cases. |
| tests/CoreTest/Configuration/UpdateOptionsStaticTests.cs | Verifies UpdateOptions static defaults and repeated-access identity. |
| tests/CoreTest/Configuration/PacketTests.cs | Covers Packet default/nullables and tri-state bool/int properties. |
| tests/CoreTest/Configuration/HubConfigTests.cs | Covers HubConfig defaults and boundary assignments. |
| tests/CoreTest/Configuration/ConfigurationMapperExtendedTests.cs | Adds mapper edge-case coverage for ConfigurationMapper methods. |
| tests/CoreTest/Configuration/ComparisonResultTests.cs | Covers ComparisonResult list accumulation and read-only list exposure. |
| tests/CoreTest/Configuration/BlackListConfigBuilderTests.cs | Covers blacklist builder fluent API, null-vs-empty behavior, and HasRules. |
| tests/CoreTest/Configuration/BaseConfigInfoTests.cs | Verifies BaseConfigInfo default values and set/get behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+15
to
+17
| private readonly ITestOutputHelper _output; | ||
|
|
||
| public DownloadProgressReporterTests(ITestOutputHelper output) => _output = output; |
Comment on lines
+19
to
+22
| public void Dispose() | ||
| { | ||
| EventManager.Instance.Clear(); | ||
| } |
Comment on lines
+14
to
+23
| public GeneralTracerTests() | ||
| { | ||
| _originalTracingEnabled = GeneralTracer.IsTracingEnabled(); | ||
| } | ||
|
|
||
| public void Dispose() | ||
| { | ||
| GeneralTracer.SetTracingEnabled(_originalTracingEnabled); | ||
| GC.SuppressFinalize(this); | ||
| } |
Comment on lines
+185
to
+192
| public void ProduceDeletes_IsSameReference_NotCopy() | ||
| { | ||
| var deleted = new[] { "x.txt" }; | ||
| var diff = Diff(deleted: deleted); | ||
|
|
||
| var result = FileTreeDiffer.ProduceDeletes(diff); | ||
|
|
||
| Assert.Same(diff.Deleted, result); |
Comment on lines
+293
to
+301
| [Fact] | ||
| public void ShouldUseDeltaPatching_NegativeTotalFiles_ReturnsTrueBecauseRatioBelowThreshold() | ||
| { | ||
| // With totalFileCount = -1, the ratio is negative, which is <= 0.5, so returns true. | ||
| // This is a known edge case — ShouldUseDeltaPatching assumes positive totalFileCount. | ||
| var diff = Diff(added: new[] { Entry("a.txt") }); | ||
| Assert.True(FileTreeDiffer.ShouldUseDeltaPatching(diff, -1)); | ||
| } | ||
|
|
Comment on lines
+170
to
+183
| public void Builder_Reused_ReadOnlyWrapsUnderlyingList() | ||
| { | ||
| // Builder caches a single _blackFiles list. Build() wraps it with AsReadOnly(), | ||
| // which returns a live view of the underlying list. Subsequent AddBlackFiles | ||
| // calls mutate that same list, so earlier builds also see the accumulated items. | ||
| var builder = new BlackListConfigBuilder(); | ||
| builder.AddBlackFiles("a.dll"); | ||
| var first = builder.Build(); | ||
| builder.AddBlackFiles("b.dll"); | ||
| var second = builder.Build(); | ||
|
|
||
| // Both see the accumulated list because ReadOnlyCollection wraps the same List<string> | ||
| Assert.Equal(2, first.BlackFiles!.Count); | ||
| Assert.Equal(2, second.BlackFiles!.Count); |
|
|
||
| var config = builder.Build(); | ||
|
|
||
| Assert.NotNull(config.BlackFiles); |
Comment on lines
+212
to
+227
| [Theory] | ||
| [InlineData(true, false, false)] | ||
| [InlineData(false, true, false)] | ||
| [InlineData(false, false, true)] | ||
| [InlineData(true, true, true)] | ||
| public void HasRules_WhenAnySectionHasItems_ReturnsTrue(bool hasFiles, bool hasFormats, bool hasDirs) | ||
| { | ||
| var builder = new BlackListConfigBuilder(); | ||
| if (hasFiles) builder.AddBlackFiles("f.dll"); | ||
| if (hasFormats) builder.AddBlackFormats(".log"); | ||
| if (hasDirs) builder.AddSkipDirectories("tmp"); | ||
|
|
||
| var config = builder.Build(); | ||
|
|
||
| Assert.True(config.HasRules); | ||
| } |
Renamed "INT_KEY" to "INT_KEY_UV" in UpdateOptionValueTests to prevent registry collision with ConfigurationModelsTests. UpdateOption.ValueOf uses a static ConcurrentDictionary — same key with different defaults causes test ordering-dependent failures on Windows CI. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Remove unused ITestOutputHelper field in DownloadProgressReporterTests - Add NonParallel_EventManager collection for EventManager/GeneralTracer tests to prevent parallel test race conditions with global singleton state - Change ProduceDeletes assertion from brittle Assert.Same to content check - Remove invalid negative totalFileCount test case from FileTreeDiffer tests - Remove leaky builder-reuse test that enshrined mutable internal state - Rename Build_ReadOnlyLists to Build_ProducesNonNullLists to match assertions - Complete HasRules theory to cover all 8 input combinations (2^3) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
27 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds 20 new test files covering all previously untested components in
GeneralUpdate.Core, following the AAA (Arrange-Act-Assert) pattern with exhaustive branch coverage.New test files
Configuration/BlackListConfigBuilderTests.csConfiguration/ComparisonResultTests.csConfiguration/UpdateOptionValueTests.csConfiguration/UpdateOptionsStaticTests.csConfiguration/HubConfigTests.csConfiguration/PacketTests.csConfiguration/VersionOSSTests.csConfiguration/VersionRespDTOTests.csConfiguration/BaseConfigInfoTests.csConfiguration/ConfigurationMapperExtendedTests.csDownload/DefaultDownloadPipelineTests.csDownload/DownloadProgressReporterTests.csDownload/DownloadModelsTests.csDownload/PacketDTOTests.csPipeline/PatchMiddlewareTests.csFileSystem/FileTreeDifferExtendedTests.csFileSystem/FileTreeSnapshotExtendedTests.csFileSystem/BlackListDefaultsTests.csTracer/GeneralTracerTests.csTracer/TextTraceListenerTests.csBranch coverage highlights
HasRulesall 8 combinations (hasFiles x hasFormats x hasDirs)ShouldUseDeltaPatchingat exact threshold, above, below, zero files, negativeTest results
Test plan
net10.0dotnet build🤖 Generated with Claude Code